-
Notifications
You must be signed in to change notification settings - Fork 33
make ssh_authorized_key readonly #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
I confirm that this approach doesn't actually work: a key created as a normal user will fail to write:
|
This is a rather bold and naive move to fix puppetlabs#92. It makes all authorized_keys generated by this module to be readonly when generated by root, so that Puppet can be used to deploy authorized_keys files that are not writable by the user, yet still usable for authentication. This is necessary because OpenSSH drops privileges before parsing authorized_keys. If a file is owned by root and mode `0600` (as right now), authentication fails. We keep the old `0600` mode for files managed by the user. For those, there's nothing we can do anyways: if the user owns the file, they can change the mode and rewrite the file anyways. A proper solution would probably be to hook into a File resource there that could be overriden properly. Fundamentally, the problem here is that we are managing multiple resources that hit the same actual file on disk: ideally, we'd have a mode parameter to the resource here, but then we could get into conflicts if multiple invocations of ssh_authorized_key use different mode parameters. Closes: puppetlabs#92
This should be fixed by the new dual approach I've taken. |
@@ -38,7 +38,11 @@ def dir_perm | |||
0o700 | |||
end | |||
|
|||
def file_perm | |||
def file_perm_readonly | |||
0o444 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know multiple organisations where world readable files aren't allowed. I don't think it's a good idea to implement such a breaking change. I think it makes more sense to add a parameter for the mode and default to 0600
for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... sure, but how would that even work considering multiple invocations of the type can have different mode parameters? how would we handle that conflict? we'd just let puppet flap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case puppet flaps or even better: the type notices it and fails.
I have just realized this is actually insufficient to make things actually work properly for us. because we manage the i think the ssh_authorized_key type needs to declare a File resource properly, and that's how ownership/mode should be managed on that file. i have, however, zero idea on how to do that so i'm not sure i'll be able to carry this forward. perhaps i should file an issue about this specifically as well... |
next step here is to autorequire a file resource (e.g. like https://github.com/puppetlabs/puppetlabs-postgresql/blob/e0486a17544ea9d253aa65af43971224c3a4cd4f/lib/puppet/type/postgresql_psql.rb#L141-L143) and make the mode a parameter to the type so it can be modified by the caller (just like the owner). that would cover most of our use cases... for now, however, i'll deploy authorized_keys in another way for my project (essentially with puppetdb queries and a simple EPP template). |
make ssh_authorized_key world-readable when deployed as root
This is a rather bold and naive move to fix #92. It makes all
authorized_keys generated by this module to be readonly when generated
by root, so that Puppet can be used to deploy authorized_keys files
that are not writable by the user, yet still usable for
authentication.
This is necessary because OpenSSH drops privileges before parsing
authorized_keys. If a file is owned by root and mode
0600
(as rightnow), authentication fails.
We keep the old
0600
mode for files managed by the user. For those,there's nothing we can do anyways: if the user owns the file, they can
change the mode and rewrite the file anyways.
A proper solution would probably be to hook into a File resource there
that could be overriden properly.
Fundamentally, the problem here is that we are managing multiple
resources that hit the same actual file on disk: ideally, we'd have a
mode parameter to the resource here, but then we could get into
conflicts if multiple invocations of ssh_authorized_key use different
mode parameters.
Closes: #92